From: Keir Fraser <keir.fraser@citrix.com>
Date: Mon, 12 May 2008 09:09:12 +0000 (+0100)
Subject: ioemu: fix disk format security vulnerability
X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~14215^2~36
X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https://%22%22/%22http:/www.example.com/cgi/%22https:/%22%22?a=commitdiff_plain;h=aa7e17e11bb354d0cda598c7e6ccff29b210eb1c;p=xen.git

ioemu: fix disk format security vulnerability

* make the xenstore reader in qemu-dm's startup determine which
  of qemu's block drivers to use according to the xenstore
  backend `type' field.  This `type' field typically comes from
  the front of the drive mapping string in ioemu.  The
  supported cases are:
    xm config file string      `type'  image format    qemu driver
     phy:[/dev/]<device>        phy     raw image       bdrv_raw
     file:<filename>            file    raw image       bdrv_raw
     tap:aio:<filename>         tap     raw image       bdrv_raw
     tap:qcow:<image>           tap     not raw         autoprobe
     tap:<cow-fmt>:<image>      tap     named format    bdrv_<cow-fmt>
  It is still necessary to autoprobe when the image is specified as
  `tap:qcow:<image>', because qemu distinguishes `qcow' and `qcow2'
  whereas blktap doesn't; `qcow' in xenstore typically means what
  qemu calls qcow2.  This is OK because qemu can safely distinguish
  the different cow formats provided we know it's not a raw image.

* Make the format autoprobing machinery never return `raw'.  This has
  two purposes: firstly, it arranges that the `tap:qcow:...' case
  above can be handled without accidentally falling back to raw
  format.  Secondly it prevents accidents in case the code changes in
  future: autoprobing will now always fail on supposed cow files which
  actually contain junk, rather than giving the guest access to the
  underlying file.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---

diff --git a/tools/ioemu/block.c b/tools/ioemu/block.c
index 16ddd1805b..7bf990f787 100644
--- a/tools/ioemu/block.c
+++ b/tools/ioemu/block.c
@@ -254,7 +254,7 @@ static BlockDriver *find_protocol(const char *filename)
 #endif
     p = strchr(filename, ':');
     if (!p)
-        return &bdrv_raw;
+        return NULL; /* do not ever guess raw, it is a security problem! */
     len = p - filename;
     if (len > sizeof(protocol) - 1)
         len = sizeof(protocol) - 1;
diff --git a/tools/ioemu/xenstore.c b/tools/ioemu/xenstore.c
index 117cc3ea1f..bba243bdd4 100644
--- a/tools/ioemu/xenstore.c
+++ b/tools/ioemu/xenstore.c
@@ -90,6 +90,7 @@ void xenstore_parse_domain_config(int hvm_domid)
     int i, is_scsi, is_hdN = 0;
     unsigned int len, num, hd_index, pci_devid = 0;
     BlockDriverState *bs;
+    BlockDriver *format;
 
     for(i = 0; i < MAX_DISKS + MAX_SCSI_DISKS; i++)
         media_filename[i] = NULL;
@@ -135,6 +136,8 @@ void xenstore_parse_domain_config(int hvm_domid)
     }
         
     for (i = 0; i < num; i++) {
+	format = NULL; /* don't know what the format is yet */
+
         /* read the backend path */
         if (pasprintf(&buf, "%s/device/vbd/%s/backend", path, e[i]) == -1)
             continue;
@@ -181,13 +184,20 @@ void xenstore_parse_domain_config(int hvm_domid)
         drv = xs_read(xsh, XBT_NULL, buf, &len);
         if (drv == NULL)
             continue;
-        /* Strip off blktap sub-type prefix aio: - QEMU can autodetect this */
+        /* Obtain blktap sub-type prefix */
         if (!strcmp(drv, "tap") && params[0]) {
             char *offset = strchr(params, ':'); 
             if (!offset)
                 continue ;
+	    free(drv);
+	    drv = malloc(offset - params + 1);
+	    memcpy(drv, params, offset - params);
+	    drv[offset - params] = '\0';
+	    if (!strcmp(drv, "aio"))
+		/* qemu does aio anyway if it can */
+		format = &bdrv_raw;
             memmove(params, offset+1, strlen(offset+1)+1 );
-            fprintf(logfile, "Strip off blktap sub-type prefix to %s\n", params); 
+            fprintf(logfile, "Strip off blktap sub-type prefix to %s (drv '%s')\n", params, drv); 
         }
         /* Prefix with /dev/ if needed */
         if (!strcmp(drv, "phy") && params[0] != '/') {
@@ -195,6 +205,7 @@ void xenstore_parse_domain_config(int hvm_domid)
             sprintf(newparams, "/dev/%s", params);
             free(params);
             params = newparams;
+	    format = &bdrv_raw;
         }
 
         /* 
@@ -240,8 +251,25 @@ void xenstore_parse_domain_config(int hvm_domid)
 #endif
 
         if (params[0]) {
-            if (bdrv_open(bs, params, 0 /* snapshot */) < 0)
-                fprintf(stderr, "qemu: could not open vbd '%s' or hard disk image '%s'\n", buf, params);
+	    if (!format) {
+		if (!drv) {
+		    fprintf(stderr, "qemu: type (image format) not specified for vbd '%s' or image '%s'\n", buf, params);
+		    continue;
+		}
+		if (!strcmp(drv,"qcow")) {
+		    /* autoguess qcow vs qcow2 */
+		} else if (!strcmp(drv,"file")) {
+		    format = &bdrv_raw;
+		} else {
+		    format = bdrv_find_format(drv);
+		    if (!format) {
+			fprintf(stderr, "qemu: type (image format) '%s' unknown for vbd '%s' or image '%s'\n", drv, buf, params);
+			continue;
+		    }
+		}
+	    }
+            if (bdrv_open2(bs, params, 0 /* snapshot */, format) < 0)
+                fprintf(stderr, "qemu: could not open vbd '%s' or hard disk image '%s' (drv '%s')\n", buf, params, drv ? drv : "?");
         }
     }